-
Notifications
You must be signed in to change notification settings - Fork 2
Make Archive TODO hotkey configurable via Roam Hotkeys #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Make Archive TODO hotkey configurable via Roam Hotkeys #11
Conversation
Replace hardcoded Cmd/Ctrl+Shift+Enter with command palette registration, allowing users to customize the shortcut in Settings → Hotkeys. - Add 'Archive TODO' command to command palette - Set default hotkey to ctrl-shift-enter for backwards compatibility - Remove unused isControl import
WalkthroughThe PR changes TODONT initialization to accept Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/todont.ts (1)
88-211: Critical: Switching between non-off modes causes duplicate registrations.When a user switches from one active TODONT mode to another (e.g., from "icon" to "strikethrough"), the existing command registration and observers are not cleaned up before the new ones are created. This leads to:
- Duplicate command palette entries
- Multiple observer instances running simultaneously
- Memory leaks as the
unloadsSet accumulates cleanup handlers🔎 Proposed fix
Add cleanup before setting up new registrations when switching modes:
const initializeTodont = (extensionAPI: OnloadArgs["extensionAPI"]) => { const unloads = new Set<() => void>(); return async (todontMode: typeof TODONT_MODES[number]) => { if (todontMode !== "off") { + // Clean up previous mode if exists + unloads.forEach((u) => u()); + unloads.clear(); + const TODONT_CLASSNAME = "roamjs-todont"; const css = document.createElement("style");
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/index.tssrc/utils/todont.ts
🔇 Additional comments (4)
src/index.ts (2)
21-21: LGTM!The updated call correctly passes
extensionAPItoinitializeTodont, enabling the command palette integration.
86-86: Good discoverability improvement!The updated description clearly informs users about the Archive TODO command and hotkey customization options, improving the feature's discoverability.
src/utils/todont.ts (2)
84-86: LGTM!The addition of the
ARCHIVE_COMMAND_LABELconstant and the updated function signature to acceptextensionAPIare well-structured changes that enable the command palette integration.
207-211: Cleanup implementation is sound.The command removal cleanup is properly structured and will work correctly once the duplicate registration issue is resolved.
| extensionAPI.ui.commandPalette.addCommand({ | ||
| label: ARCHIVE_COMMAND_LABEL, | ||
| callback: todontCallback, | ||
| "default-hotkey": "ctrl-shift-enter", | ||
| "disable-hotkey": false, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
rg -n "commandPalette\.addCommand" -A 5 -B 2Repository: RoamJS/todo-trigger
Length of output: 961
🏁 Script executed:
rg -n "default-hotkey|disable-hotkey" -B 3 -A 3Repository: RoamJS/todo-trigger
Length of output: 546
🏁 Script executed:
rg -n "ctrl-|shift-|meta-" -i --type ts --type js | head -20Repository: RoamJS/todo-trigger
Length of output: 132
🏁 Script executed:
find . -name "*.d.ts" -o -name "README*" -o -name "*roam*" -type f | grep -v node_modules | head -20Repository: RoamJS/todo-trigger
Length of output: 74
🏁 Script executed:
cat package.json | grep -A 5 -B 5 roamRepository: RoamJS/todo-trigger
Length of output: 549
🏁 Script executed:
rg -n "roamAlphaAPI|roam.*API" -i | head -30Repository: RoamJS/todo-trigger
Length of output: 1110
🏁 Script executed:
find . -path "*/roamjs-components/*" -name "*.d.ts" | head -20Repository: RoamJS/todo-trigger
Length of output: 45
🏁 Script executed:
grep -r "commandPalette" . --include="*.d.ts" 2>/dev/null | head -20Repository: RoamJS/todo-trigger
Length of output: 45
🏁 Script executed:
head -100 README.mdRepository: RoamJS/todo-trigger
Length of output: 3738
🌐 Web query:
Roam Research command palette hotkey format ctrl shift enter API documentation
💡 Result:
Use a plus-separated, lowercase modifier+key string (e.g. "ctrl+shift+enter" or "command+shift+p"). Libraries and apps that Roam integrations use follow this format (modifiers first, joined with "+", then the key). Examples: "ctrl+shift+enter", "ctrl+shift+p". [1][2]
Sources:
- Roam shortcut listings showing modifier+key notation. [1]
- npm command-palette component (hotKeys default format "command+shift+p"). [2]
- TestCafe / common tooling examples using "ctrl+shift+enter" style. [3]
References:
[1] Roam Research keyboard shortcuts (cheatsheet).
[2] @folk-org/react-command-palette (npm) — hotKeys format example.
[3] TestCafe / Stack Overflow example using 'ctrl+shift+enter'.
Fix the hotkey format: use plus signs instead of hyphens.
The hotkey should be "ctrl+shift+enter" not "ctrl-shift-enter". The README documents this for users as "CMD+SHIFT+ENTER (CTRL in windows)", and standard keyboard shortcut notation uses plus signs (+) to separate modifiers and keys. The current hyphen-separated format will likely fail to register the hotkey properly.
🤖 Prompt for AI Agents
In src/utils/todont.ts around lines 201 to 206 the command palette hotkey is
using hyphens ("ctrl-shift-enter") which is incorrect; update the
"default-hotkey" value to use plus signs ("ctrl+shift+enter") so the
modifier/key format matches the README and will register correctly in the UI;
keep the rest of the command object unchanged.
Move cleanup to start of mode toggle function to ensure previous mode's command and observers are cleaned up before new ones are registered. This prevents duplicate command palette entries and memory leaks when switching between icon/strikethrough modes.
Summary
ctrl-shift-enterfor backwards compatibilityTest Plan
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.